Versal SDCard support #678

dgarske · 2026-01-30T00:13:04Z

Versal SD card boot support with MBR parsing, SDHCI translation for Arasan, and polling-mode operation.
Disk boot payload loading fix to load firmware directly at WOLFBOOT_LOAD_ADDRESS (no post‑load memmove).
AArch64 startup/debug improvements including RVBAR skip on Versal and optional DEBUG_HARDFAULT EL2 exception dumps.
Reproducible build banner support (suppresses __DATE__/__TIME__ when enabled).
Updated Versal documentation and test flow for QSPI vs SD card setup and provisioning.

Copilot

Pull request overview

Adds AMD Versal VMK180 SD-card boot support by enabling disk-based image loading from MBR partitions and providing the required SDHCI (Arasan) integration, plus related build/docs/test-flow updates.

Changes:

Add MBR fallback parsing in the disk layer and update the disk boot loader to load payloads directly at WOLFBOOT_LOAD_ADDRESS.
Add Versal SDHCI register translation + polling support and extend AArch64 startup/debug behavior for Versal.
Add SD-card configuration, documentation, and a test script flow to build/sign images and generate an SD-card image.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
tools/scripts/versal_test.sh	Adds `--sdcard` / `--linux-sdcard` flows and helpers to create an MBR-partitioned SD image and write artifacts into partitions.
test-app/app_versal.c	Gates fixed-partition version reporting behind `WOLFBOOT_FIXED_PARTITIONS`; adds messaging for disk-based boot.
src/update_disk.c	Changes disk loading to skip the header on read, decrypt payload only, and avoid post-load moves.
src/sdhci.c	Improves SDMA interrupt handling and adds polling-mode IRQ waiting; adds an option to disable SDMA.
src/disk.c	Adds MBR partition table parsing fallback when GPT protective MBR isn’t present.
src/boot_aarch64_start.S	Adds `wfi` to the error loop to reduce busy spinning.
src/boot_aarch64.c	Adds optional EL2 exception register dumps under `DEBUG_HARDFAULT`.
hal/versal.h	Adds SDIO ref-clock/reset register definitions.
hal/versal.c	Adds reproducible build banner option, disables flash-partition getters when `WOLFBOOT_NO_PARTITIONS`, and implements Versal SDHCI offset translation + polling init.
docs/Targets.md	Updates Versal target docs for QSPI vs SD boot paths and provisioning steps.
config/examples/versal_vmk180_sdcard.config	Introduces an SD-card boot config using disk partitions (MBR A/B layout).
arch.mk	Selects disk-based updater + disk/GPT objects for AARCH64 when SD/eMMC is enabled; adds Versal-specific flags (RVBAR skip, SDMA disable).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tools/scripts/versal_test.sh

src/update_disk.c

src/disk.c

config/examples/versal_vmk180_sdcard.config

tools/scripts/versal_test.sh

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 9 comments.

Comments suppressed due to low confidence (1)

src/disk.c:123

Bounds check in disk_open(): if ((drv < 0) || (drv > MAX_DISKS)) allows drv==MAX_DISKS, which will index Drives[MAX_DISKS] out of bounds. This should be drv >= MAX_DISKS (and similarly in other drive/partition checks) to match the 0..MAX_DISKS-1 valid range.

    if ((drv < 0) || (drv > MAX_DISKS)) {
        wolfBoot_printf("Attempting to access invalid drive %d\r\n", drv);
        return -1;
    }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-04T18:53:06Z

hal/versal.c

+/* Use SD1 for external SD card slot on VMK180 */
+#define VERSAL_SDHCI_BASE  VERSAL_SD1_BASE  /* 0xF1050000 */


This HAL compiles for both DISK_SDCARD and DISK_EMMC, but VERSAL_SDHCI_BASE is hard-coded to SD1 (external SD slot). If DISK_EMMC is enabled, the controller is typically SD0 (VERSAL_SD0_BASE). Consider selecting the base address based on is_emmc in sdhci_platform_set_bus_mode() (or compile-time via DISK_EMMC) so eMMC builds don't accidentally talk to the SD1 controller.

Suggested change

/* Use SD1 for external SD card slot on VMK180 */

#define VERSAL_SDHCI_BASE VERSAL_SD1_BASE /* 0xF1050000 */

/* Select SD controller base depending on storage type:

* - eMMC typically uses SD0 (internal)

* - External SD card slot (VMK180) uses SD1

*/

#if defined(DISK_EMMC)

#define VERSAL_SDHCI_BASE VERSAL_SD0_BASE

#elif defined(DISK_SDCARD)

#define VERSAL_SDHCI_BASE VERSAL_SD1_BASE /* 0xF1050000 */

#endif

Copilot · 2026-02-04T18:53:07Z

tools/scripts/versal_test.sh

+# SD card image configuration
+SDCARD_IMG="${SDCARD_IMG:-${WOLFBOOT_ROOT}/sdcard.img}"
+SDCARD_SIZE_MB="${SDCARD_SIZE_MB:-512}"
+SDCARD_BOOT_SIZE_MB="${SDCARD_BOOT_SIZE_MB:-128}"
+SDCARD_OFP_SIZE_MB="${SDCARD_OFP_SIZE_MB:-200}"


Default SD card image size (SDCARD_SIZE_MB=512) is smaller than the partition layout created by create_sdcard_image() (128MB boot + 200MB OFP_A + 200MB OFP_B = 528MB before partition 4). With defaults, sfdisk will fail or create a truncated layout. Increase the default image size (e.g., 1024MB) and/or validate that SDCARD_SIZE_MB >= SDCARD_BOOT_SIZE_MB + 2*SDCARD_OFP_SIZE_MB (+ headroom) before calling sfdisk.

Copilot · 2026-02-04T18:53:07Z

tools/scripts/versal_test.sh

+    # Parse sfdisk dump for partition start sector
+    sector=$(sfdisk -d "$img" 2>/dev/null | grep "^${img}${part}" | sed 's/.*start=[[:space:]]*\([0-9]*\).*/\1/')
+    if [ -z "$sector" ]; then
+        # Fallback: parse fdisk output
+        sector=$(fdisk -l "$img" 2>/dev/null | grep "^${img}${part}" | awk '{print $2}')
+        [ "$sector" = "*" ] && sector=$(fdisk -l "$img" 2>/dev/null | grep "^${img}${part}" | awk '{print $3}')


get_partition_offset() builds a grep regex from $img (e.g., sdcard.img), so characters like '.' are treated as regex wildcards. This can mis-match the sfdisk/fdisk output and return an incorrect start sector. Use grep -F (fixed-string) or escape $img before using it in a regex.

Suggested change

# Parse sfdisk dump for partition start sector

sector=$(sfdisk -d "$img" 2>/dev/null | grep "^${img}${part}" | sed 's/.*start=[[:space:]]*$[0-9]*$.*/\1/')

if [ -z "$sector" ]; then

# Fallback: parse fdisk output

sector=$(fdisk -l "$img" 2>/dev/null | grep "^${img}${part}" | awk '{print $2}')

[ "$sector" = "*" ] && sector=$(fdisk -l "$img" 2>/dev/null | grep "^${img}${part}" | awk '{print $3}')

# Escape regex metacharacters in img before using it in grep patterns

local img_escaped

img_escaped=$(printf '%s\n' "$img" | sed 's/[][\\.^$*+?(){|}/\\&/g')

# Parse sfdisk dump for partition start sector

sector=$(sfdisk -d "$img" 2>/dev/null | grep "^${img_escaped}${part}" | sed 's/.*start=[[:space:]]*$[0-9]*$.*/\1/')

if [ -z "$sector" ]; then

# Fallback: parse fdisk output

sector=$(fdisk -l "$img" 2>/dev/null | grep "^${img_escaped}${part}" | awk '{print $2}')

[ "$sector" = "*" ] && sector=$(fdisk -l "$img" 2>/dev/null | grep "^${img_escaped}${part}" | awk '{print $3}')

Copilot · 2026-02-04T18:53:07Z

src/disk.c

+        {
+            uint32_t n = drive->n_parts;
+            uint64_t start_bytes = (uint64_t)pte->lba_first * GPT_SECTOR_SIZE;
+            uint64_t end_bytes = start_bytes +
+                ((uint64_t)pte->lba_size * GPT_SECTOR_SIZE) - 1;
+
+            drive->part[n].drv = drive->drv;
+            drive->part[n].start = start_bytes;
+            drive->part[n].end = end_bytes;
+            memset(drive->part[n].name, 0, sizeof(drive->part[n].name));
+            drive->n_parts++;
+


disk_open_mbr() increments drive->n_parts and writes drive->part[n] without checking that n < MAX_PARTITIONS. If MAX_PARTITIONS is configured < 4, this will write out of bounds. Add a bounds check (and return an error) when drive->n_parts reaches MAX_PARTITIONS.

Copilot · 2026-02-04T18:53:07Z

docs/Targets.md

 source ${VITIS_PATH}/settings64.sh
 bootgen -arch versal -image ./tools/scripts/vmk180/boot_wolfboot.bif -w -o BOOT.BIN
 ```

-The BIF file (`boot_wolfboot.bif`) references files using relative paths in the same directory. After successful generation, `BOOT.BIN` will be created in `tools/scripts/vmk180/`.
+The BIF file (`boot_wolfboot.bif`) references files using relative paths in the same directory.


The documented bootgen command references ./tools/scripts/vmk180/boot_wolfboot.bif, but that path doesn't exist in the repo (only tools/scripts/versal_boot.bif is present). Update the documentation (and the following sentence) to point at the correct BIF file so users can reproduce the BOOT.BIN generation steps.

Copilot · 2026-02-04T18:53:08Z

src/update_disk.c

        do {
-            ret = disk_part_read(BOOT_DISK, cur_part, load_off,
-                DISK_BLOCK_SIZE, ((uint8_t *)load_address) + load_off);
+            uint32_t chunk = os_image.fw_size - load_off;
+            if (chunk > DISK_BLOCK_SIZE)
+                chunk = DISK_BLOCK_SIZE;
+            ret = disk_part_read(BOOT_DISK, cur_part,
+                IMAGE_HEADER_SIZE + load_off, chunk,
+                ((uint8_t *)load_address) + load_off);
            if (ret < 0)
                break;
            load_off += ret;
-        } while (load_off < os_image.fw_size + IMAGE_HEADER_SIZE);
+        } while (load_off < os_image.fw_size);


The payload load loop assumes disk_part_read() always returns a positive byte count when called with chunk > 0. If disk_part_read() ever returns 0 (e.g., due to partition boundary truncation/off-by-one), load_off will not advance and this becomes an infinite loop. Treat ret <= 0 as an error (break/fallback) and consider validating that os_image.fw_size fits within the selected partition before starting the load.

Copilot · 2026-02-04T18:53:08Z

tools/scripts/versal_test.sh

+write_to_partition() {
+    local img="$1" part="$2" file="$3"
+    local offset_bytes=$(get_partition_offset "$img" "$part")
+    local offset_blocks=$((offset_bytes / 512))
+
+    if [ -z "$offset_blocks" ] || [ "$offset_blocks" -eq 0 ]; then
+        log_error "Failed to get partition $part offset"
+        return 1
+    fi
+
+    log_info "Writing $file to partition $part (offset: ${offset_bytes} bytes, sector: ${offset_blocks})"
+    dd if="$file" of="$img" bs=512 seek="$offset_blocks" conv=notrunc status=progress 2>/dev/null || {
+        log_error "Failed to write $file to partition $part"
+        return 1
+    }


write_to_partition() dd's the full file into the image at the partition start but doesn't verify that the file fits within the target partition size. If an image is larger than the partition, this will silently overwrite subsequent partitions. Consider parsing the partition size from sfdisk output and failing early when file_size > part_size.

Copilot · 2026-02-04T18:53:08Z

tools/scripts/versal_test.sh

+        # Write rootfs to partition 4 if available
+        ROOTFS_IMG=""
+        if [ -f "${LINUX_IMAGES_DIR}/rootfs.ext4" ]; then
+            ROOTFS_IMG="${LINUX_IMAGES_DIR}/rootfs.ext4"
+        elif [ -f "${LINUX_IMAGES_DIR}/rootfs.cpio.gz" ]; then
+            ROOTFS_IMG="${LINUX_IMAGES_DIR}/rootfs.cpio.gz"
+        fi
+        if [ -n "$ROOTFS_IMG" ]; then
+            log_info "Writing rootfs to partition 4..."
+            write_to_partition "$SDCARD_IMG" 4 "$ROOTFS_IMG" || exit 1
+            log_ok "rootfs written ($(stat -c%s "$ROOTFS_IMG") bytes)"
+        else
+            log_info "No rootfs found in $LINUX_IMAGES_DIR (looked for rootfs.ext4, rootfs.cpio.gz)"
+            log_info "You can write rootfs to partition 4 manually"


Writing rootfs.cpio.gz directly into partition 4 via dd won't produce a usable root filesystem partition (cpio archive != block filesystem image). Consider only auto-writing rootfs.ext4 here, and if the intent is initramfs, include rootfs.cpio.gz in the FIT image instead (or clearly gate this behavior behind an explicit flag).

Suggested change

# Write rootfs to partition 4 if available

ROOTFS_IMG=""

if [ -f "${LINUX_IMAGES_DIR}/rootfs.ext4" ]; then

ROOTFS_IMG="${LINUX_IMAGES_DIR}/rootfs.ext4"

elif [ -f "${LINUX_IMAGES_DIR}/rootfs.cpio.gz" ]; then

ROOTFS_IMG="${LINUX_IMAGES_DIR}/rootfs.cpio.gz"

fi

if [ -n "$ROOTFS_IMG" ]; then

log_info "Writing rootfs to partition 4..."

write_to_partition "$SDCARD_IMG" 4 "$ROOTFS_IMG" || exit 1

log_ok "rootfs written ($(stat -c%s "$ROOTFS_IMG") bytes)"

else

log_info "No rootfs found in $LINUX_IMAGES_DIR (looked for rootfs.ext4, rootfs.cpio.gz)"

log_info "You can write rootfs to partition 4 manually"

# Write rootfs filesystem image to partition 4 if available

ROOTFS_IMG=""

if [ -f "${LINUX_IMAGES_DIR}/rootfs.ext4" ]; then

ROOTFS_IMG="${LINUX_IMAGES_DIR}/rootfs.ext4"

fi

if [ -n "$ROOTFS_IMG" ]; then

log_info "Writing rootfs to partition 4..."

write_to_partition "$SDCARD_IMG" 4 "$ROOTFS_IMG" || exit 1

log_ok "rootfs written ($(stat -c%s "$ROOTFS_IMG") bytes)"

else

log_info "No rootfs filesystem image found in $LINUX_IMAGES_DIR (looked for rootfs.ext4)"

log_info "You can write a rootfs filesystem image to partition 4 manually"

Copilot · 2026-02-04T18:53:08Z

src/sdhci.c

+            /* SDMA mode with Host Version 4 enable.
+             * HV4E is required for SDMA to use the 64-bit address registers
+             * (SRS22/SRS23) instead of the legacy 32-bit register (SRS00).
+             * A64S is cleared to use 32-bit DMA addressing. */
            sdhci_reg_or(SDHCI_SRS10, SDHCI_SRS10_DMA_SDMA);
-            if (dir == SDHCI_DIR_WRITE) {
-                sdhci_reg_or(SDHCI_SRS15, SDHCI_SRS15_HV4E);
-                sdhci_reg_and(SDHCI_SRS16, ~SDHCI_SRS16_A64S);
-            }
+            sdhci_reg_or(SDHCI_SRS15, SDHCI_SRS15_HV4E);
+            sdhci_reg_and(SDHCI_SRS16, ~SDHCI_SRS16_A64S);


In the SDMA setup, the code writes to SDHCI_SRS16 to clear SDHCI_SRS16_A64S. SDHCI_SRS16 is defined as Capabilities 1 (typically read-only) and A64S is a capability bit, not a control bit. If the goal is to force 32-bit addressing, the control bit to clear is SDHCI_SRS15_A64 in Host Control 2 (or just omit forcing it and rely on controller capabilities). Writing to the capabilities register may be ignored or behave unexpectedly on some IP.

dgarske self-assigned this Jan 30, 2026

dgarske force-pushed the versal_sdcard branch 3 times, most recently from 9a23340 to 1eac275 Compare February 2, 2026 22:51

dgarske assigned danielinux and wolfSSL-Bot Feb 2, 2026

dgarske requested a review from danielinux February 2, 2026 22:51

dgarske force-pushed the versal_sdcard branch from 1eac275 to 20d6015 Compare February 2, 2026 23:10

Versal SDCard support

3eae2f3

dgarske force-pushed the versal_sdcard branch from 20d6015 to 3eae2f3 Compare February 3, 2026 18:33

dgarske removed their assignment Feb 4, 2026

danielinux previously approved these changes Feb 4, 2026

View reviewed changes

danielinux unassigned wolfSSL-Bot Feb 4, 2026

danielinux requested a review from Copilot February 4, 2026 16:18

Copilot started reviewing on behalf of danielinux February 4, 2026 16:19 View session

Copilot AI reviewed Feb 4, 2026

View reviewed changes

danielinux assigned dgarske and unassigned danielinux Feb 4, 2026

Peer review fixes

7e9d65f

dgarske dismissed danielinux’s stale review via 7e9d65f February 4, 2026 18:12

dgarske requested a review from Copilot February 4, 2026 18:16

Copilot started reviewing on behalf of dgarske February 4, 2026 18:19 View session

Copilot AI reviewed Feb 4, 2026

View reviewed changes

		/* Use SD1 for external SD card slot on VMK180 */
		#define VERSAL_SDHCI_BASE VERSAL_SD1_BASE /* 0xF1050000 */

-/* Use SD1 for external SD card slot on VMK180 */
-#define VERSAL_SDHCI_BASE  VERSAL_SD1_BASE  /* 0xF1050000 */
+/* Select SD controller base depending on storage type:
+ *   - eMMC typically uses SD0 (internal)
+ *   - External SD card slot (VMK180) uses SD1
+ */
+#if defined(DISK_EMMC)
+#define VERSAL_SDHCI_BASE  VERSAL_SD0_BASE
+#elif defined(DISK_SDCARD)
+#define VERSAL_SDHCI_BASE  VERSAL_SD1_BASE  /* 0xF1050000 */
+#endif

Versal SDCard support #678

Are you sure you want to change the base?

Versal SDCard support #678

Uh oh!

Conversation

dgarske commented Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dgarske commented Jan 30, 2026 •

edited

Loading